Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Shortcodes: update MailChimp shortcode to match new format. #10105

Merged
merged 2 commits into from
Sep 21, 2018
Merged

Shortcodes: update MailChimp shortcode to match new format. #10105

merged 2 commits into from
Sep 21, 2018

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Sep 6, 2018

Fixes #10104

Changes proposed in this Pull Request:

Mailchimp updated their newsletter embed code, and the old one does not work anymore.
This change matches the new format, and updates our test accordingly.

Note that this also reverts the changes introduced in #8547, since they do not appear necessary anymore. Mailchimp fixed that issue on their end.

Testing instructions:

Try adding Maichimp shortcodes to your site in different ways:

  • As an admin, add the new embed code to a new post.
  • As a contributor, add the new embed code to a new post and save your draft. Watch the code convert into a shortcode.
  • Make sure the newsletter pop up appears on your site when you view the post.
  • Make sure no JavaScript errors appear in your browser console.

Here is an example embed code you can add to your post:

<script type="text/javascript" src="//downloads.mailchimp.com/js/signup-forms/popup/unique-methods/embed.js" data-dojo-config="usePlainJson: true, isDebug: false"></script><script type="text/javascript">window.dojoRequire(["mojo/signup-forms/Loader"], function(L) { L.start({"baseUrl":"mc.us8.list-manage.com","uuid":"be06c2a596db91bfe4099fde8","lid":"08cf5fa008","uniqueMethods":true}) })</script>

Proposed changelog entry for your changes:

  • Shortcodes: update Mailchimp shortcode to match the new format offered by Mailchimp.

@jeherve jeherve requested a review from a team as a code owner September 6, 2018 08:21
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Shortcodes / Embeds [Status] Needs Review This PR is ready for review. [Pri] High labels Sep 6, 2018
@jeherve jeherve added this to the 6.6 milestone Sep 6, 2018
@jetpackbot
Copy link
Collaborator

That's a great PR description, thank you so much for your effort!

Generated by 🚫 dangerJS

jeherve added a commit that referenced this pull request Sep 6, 2018
@jeherve
Update testing list to include steps for #10105
42619cd
@jeherve jeherve self-assigned this Sep 6, 2018
@BrookeDot BrookeDot changed the title Shortcodes: update Mailchimp shortcode to match new format. Shortcodes: update MailChimp shortcode to match new format. Sep 8, 2018
@brbrr
Copy link
Contributor

brbrr commented Sep 14, 2018

I've tested with few shortcodes and different user roles. LFTM 🐑

@jeherve
Shortcodes: update Mailchimp shortcode to match new format.
2fb58e1 
Fixes #10104

Mailchimp updated their newsletter embed code, and the old one does not work anymore.
This change matches the new format, and updates our test accordingly.

Note that this also reverts the changes introduced in #8547, since they do not appear necessary anymore. Mailchimp fixed that issue on their end.
@jeherve
Update testing list to include steps for #10105
3d285ce
@jeherve jeherve force-pushed the fix/shortcode-mailchimp-update-10104 branch from 42619cd to 3d285ce Compare September 20, 2018 15:46
oskosk
oskosk approved these changes Sep 21, 2018
View reviewed changes
Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this one. Works great. But couldn't reproduce the issue. As discussed on Slack, it may be that Mailchimp is handling backwards compatibility after their own change. I'd merge this PR as it seems to represent current state of affairs.

@oskosk oskosk added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Sep 21, 2018
@kraftbj kraftbj merged commit e572356 into master Sep 21, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Sep 21, 2018
@kraftbj kraftbj deleted the fix/shortcode-mailchimp-update-10104 branch September 21, 2018 14:19
@jeherve jeherve added [Status] Needs Testing We need to add this change to the testing call for this month's release [Status] Has Changelog and removed [Status] Needs Changelog labels Sep 21, 2018
@jeherve
Copy link
Member Author

jeherve commented Sep 24, 2018

Porting those changes back to WordPress.com in D18721-code.

jeherve added a commit that referenced this pull request Sep 24, 2018
@jeherve
changelog: add #10105
7aeec3b
jeherve added a commit that referenced this pull request Sep 25, 2018
@jeherve
Changelog for 6.6 release (#10107)
581489a 
* Readme: add boilerplate for next release, 6.6

* Add 6.5 to the changelog.txt file

* Set boilerplate testing list for 6.6

* Readme: update stable tag to 6.5

* Add bullets to 6.5 changelog items

* Readme: add link to previous changelogs

This will help folks who want to know more about past releases,
while keeping the readme.txt short so as to not overwhelm translators and site owners only looking for information about the last release.

* Changelog: add information at the top of the changelog file.

* Changelog: add #10054

* Changelog: add #10078

* Changelog: add #10079

* Changelog: add #10064

* Changelog: add #10094

* Changelog: add #10096

* Testing list: add more information based on #10087

* Changelog: add #9847

* Changelog: add #10084

* Changelog: add #9918

* Changelog: add #7614

* Changelog: add #10116

* Changelog: add #10108

* Changelog: add #10041

* Changelog: add #10121

* Changelog: add #10134

* Changelog: add #10130

* Changelog: add #10109

* changelog: add #10137

* changelog: add #9952

* changelog: add #10120

* changelog: add #10162

* Changelog: add #10163

* Changelog: add #10092

* changelog: add #10156

* Changelog: add #10154

* changelog: add #10122

* Changelog: add #10101

* changelog: add #10105

* changelog: add #10190

* Changelog: add #10196

* changelog: add #10152

* Changelog: add #10153

* Testing list: add more details to Site Verification testing steps.

@see #10143 (comment)

* changelog: add #10194

* Changelog: add #10193
@nagpai
Copy link

nagpai commented Oct 10, 2018

Does not seem to be working yet on WP.com
#7115384-hc

@jeherve
Copy link
Member Author

jeherve commented Oct 10, 2018

@nagpai The change was just pushed to WordPress.com as well. The user should be all set now.

@jeherve jeherve removed the [Status] Needs Testing We need to add this change to the testing call for this month's release label Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oskosk oskosk oskosk approved these changes

@brbrr brbrr brbrr approved these changes

Assignees

@jeherve jeherve

Labels
[Feature] Shortcodes / Embeds [Pri] High Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
6.6
Development

Successfully merging this pull request may close these issues.

MailChimp Widget: Can't Convert Updated Popup Code
7 participants
@jeherve @jetpackbot @brbrr @nagpai @oskosk @kraftbj @matticbot